Skip to content

Conversation

@khwilliamson
Copy link
Contributor

This is simpler and saves a malloc each time.

Note that this code could use plain utf8_to_bytes() on older perls, but it is less convenient, so would require more code; I don't think the performance gain is worth it.

  • This set of changes does not require a perldelta entry.


/* If we are able to downgrade here; that means that we have a
* key which only had chars 0-255, but was utf8 encoded. */
if (utf8_to_bytes_overwrite( (U8**) &keyval, &keylen_tmp)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a bit worried about this, since this modifies the PV belonging to a SV, but the SV is a copy of the key from the hash on this branch, so it's safe.

It does make a UTF-8 SV with a validly encoded PV into a UTF-8 SV with a probably invalidly encoded PV, but that SV is almost immediately discarded.

Will the new utf8_to_bytes_* functions be added to ppport.h?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@khwilliamson could you respond to @tonycoz's concerns above? Thx.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes they will

@khwilliamson khwilliamson added the defer-next-dev This PR should not be merged yet, but await the next development cycle label Apr 24, 2025
This is simpler and saves a malloc each time.

Note that this code could use plain utf8_to_bytes() on older perls, but
it is less convenient, so would require more code; I don't think the
performance gain is worth it.
@khwilliamson khwilliamson merged commit 023bce4 into Perl:blead Sep 16, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

defer-next-dev This PR should not be merged yet, but await the next development cycle

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants